ci: replace cpp-linter-action with cpp-linter CLI by LuciferYang · Pull Request #838 · apache/iceberg-cpp

LuciferYang · 2026-07-22T06:33:36Z

What

cpp-linter/cpp-linter-action is frozen at v2.15.1. Its v2.16 release pulled in an untrusted dependency and was blocked by ASF Infra (apache/infrastructure-actions#325), and the ASF gateway ignores newer versions. The pinned action still runs, but it can no longer be updated.

How

The action is a thin wrapper around the cpp-linter PyPI package, and the block does not touch that package. This installs it with pip and calls the cpp-linter CLI to run clang-tidy. Because it is a run: step and not a uses: reference, it adds no new third-party action to the ASF allowlist. The checks-failed output and the Fail fast step stay as they were.

Formatting is left to the pre-commit workflow, which already runs the pinned clang-format hook on every PR. So cpp-linter runs tidy-only: --style='', and clang-format is not installed here. This keeps the clang-format version in one place instead of tracking it in two. --version points at the setup-python bin directory so clang-tidy resolves to the pip-installed version rather than a system clang-tidy on PATH.

I also looked at emitting SARIF and uploading it with github/codeql-action/upload-sarif, which is allowed since it is a github/* action. That route loses the thread comment and needs an extra conversion step, so calling the CLI directly stays closer to what we have now.

Verified

CI ran the job on a temporary probe file seeded with a NULL and a mis-named member. clang-tidy resolved to the pip-installed 22.1.8 (not a system clang-tidy-22), flagged both issues, and set checks-failed, so the Fail fast step failed as expected. The probe has since been removed.

This contribution was authored with assistance from Claude Opus 4.8, in line with the Iceberg guidelines for AI-assisted contributions (https://iceberg.apache.org/contribute/#guidelines-for-ai-assisted-contributions). All changes were reviewed and tested by the author.

Closes #336

cpp-linter/cpp-linter-action is frozen at v2.15.1: v2.16+ introduced an untrusted dependency and was blocked by ASF Infra (apache/infrastructure-actions#325), and the ASF gateway ignores newer versions. The pinned action still runs but can no longer be updated. The action is a thin wrapper around the cpp-linter PyPI package, which is not affected by the block. Install that package plus matching clang tools with pip and call the cpp-linter CLI with the same options the action used, keeping the checks-failed output and the Fail fast step. This is a run: step rather than a uses: reference, so it adds no new third-party action to the ASF allowlist. Fixes apache#336.

Copilot

Pull request overview

This PR updates the C++ linter workflow to stop using the frozen cpp-linter/cpp-linter-action wrapper and instead install and run the cpp-linter PyPI CLI directly, keeping the existing “checks-failed” / fail-fast behavior while avoiding adding a new third‑party action reference.

Changes:

Replace cpp-linter/cpp-linter-action with a setup-python + pip install + cpp-linter CLI invocation in the CI workflow.
Add a temporary probe C++ source file intended to intentionally trigger clang-tidy/naming warnings to exercise the new linter path.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
`src/iceberg/temp_linter_probe.cc`	Adds a temporary intentionally-noncompliant file to force the linter to report findings.
`.github/workflows/cpp-linter.yml`	Switches from the GitHub Action wrapper to installing and running the `cpp-linter` CLI with equivalent arguments.

LuciferYang · 2026-07-22T06:41:46Z

+// TEMP verification file — will be dropped before merge.
+// Intentionally violates .clang-tidy to confirm the new cpp-linter CLI runs.


This is a temporary test for lint validation and will be deleted after testing.

It looks like the check is taking effect. I will revert this file and run the tests again.

LuciferYang · 2026-07-22T06:58:43Z

+      - uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6.3.0
+        if: github.event_name == 'pull_request'
+      - name: Install cpp-linter and clang tools
+        if: github.event_name == 'pull_request'
+        run: pip install "cpp-linter==1.13.0" "clang-format==22.1.8" "clang-tidy==22.1.8"


Good catch, done. Pinned setup-python to 3.13 and switched to python -m pip install so it uses the interpreter the action set up. I kept the run step as cpp-linter (its console entry point) since the package has no __main__, so it can't be called with python -m; it's installed into the same interpreter, so they stay in sync.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

.github/workflows/cpp-linter.yml:98

actions/setup-python is invoked without specifying python-version, which makes the job dependent on the runner’s preinstalled Python (and which pip it provides). That can change over time and can also result in pip resolving to a different interpreter than the one setup-python put on PATH. Specify a Python version (consistent with other workflows) and use python -m pip for the installs.

      - uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6.3.0
        if: github.event_name == 'pull_request'
      - name: Install cpp-linter and clang tools
        if: github.event_name == 'pull_request'
        run: pip install "cpp-linter==1.13.0" "clang-format==22.1.8" "clang-tidy==22.1.8"

Address review feedback: pin setup-python to 3.13 so the runner's default Python cannot drift, and install with python -m pip so it targets the interpreter the action set up rather than a bare pip on PATH.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

LuciferYang · 2026-07-22T07:35:26Z

cc @wgtmac

wgtmac

Nice job! Thanks a lot, @LuciferYang!

wgtmac · 2026-07-22T07:59:12Z

+          python-version: '3.13'
+      - name: Install cpp-linter and clang tools
+        if: github.event_name == 'pull_request'
+        run: python -m pip install "cpp-linter==1.13.0" "clang-format==22.1.8" "clang-tidy==22.1.8"


Should we parameterize these versions somewhere?

Done. The versions now sit in the job's env block (CPP_LINTER_VERSION, CLANG_TOOLS_VERSION), and --version takes its major from ${CLANG_TOOLS_VERSION%%.*}, so the clang version is only written once.

I left one thing out: .pre-commit-config.yaml pins its own clang-format (v22.1.5), so it and the CI job aren't sharing a source. Actions has no way to read a version across files without extra tooling, so I didn't want to widen this PR for it. I can open a separate issue if aligning them is worth it.

Since .pre-commit-config.yaml pins clang-format to 22.1.5, shouldn't we use the same version here?

Let me test this out.

Good point on aligning. One wrinkle: clang-format 22.1.5 is on PyPI, but clang-tidy skips it (only 22.1.0, 22.1.0.1, 22.1.7, and 22.1.8 are published), so I can't pin both to 22.1.5 from a single variable without breaking the tidy install.

Two ways to handle it: split into CLANG_FORMAT_VERSION (22.1.5, matching pre-commit) and CLANG_TIDY_VERSION (nearest available, 22.1.8); or bump pre-commit's clang-format to 22.1.8 so all three land on one version. pre-commit only runs clang-format here, not clang-tidy, so the alignment really only concerns the format version. Which do you prefer?

Sorry I clicked the suggestion by accident, now I've reverted it.

I would prefer decoupling them into CLANG_FORMAT_VERSION and CLANG_TIDY_VERSION first because they may diverge in the future anyway. I'm fine to choose whatever version. We need to add a comment on either side to notify they should be in sync. WDYT?

Split them as you suggested: CLANG_FORMAT_VERSION (22.1.5, matching pre-commit) and CLANG_TIDY_VERSION (22.1.8, since clang-tidy has no 22.1.5 on PyPI). Added a comment on the format version pointing at .pre-commit-config.yaml as the sync reminder.

Before we settle on 22.1.5, one thing I hit while testing the pin. Running clang-format 22.1.5 (the version pre-commit pins) over the current tree, 4 files come back as non-conforming: catalog/rest/error_handlers.cc, error_handlers.h, test/error_handlers_test.cc, and util/error_collector.h. They pass under 22.1.8 (what CI runs today), but 22.1.5 wants to reformat them.

So the tree is effectively formatted to 22.1.8 right now, while pre-commit declares 22.1.5. The two are already out of sync. If CI's clang-format goes to 22.1.5, any later PR touching those files would fail the linter on formatting its author never changed.

I'd lean toward putting both CI and pre-commit on 22.1.8 instead: CLANG_FORMAT_VERSION, CLANG_TIDY_VERSION, and pre-commit all land on one version, and nothing needs reformatting. Same "keep them in sync" outcome you asked for, without the churn. WDYT?

Move the cpp-linter and clang tool versions into the job's env block so they live in one place, and derive the clang-tidy --version major from CLANG_TOOLS_VERSION instead of repeating it.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

+      - name: Install cpp-linter and clang tools
+        if: github.event_name == 'pull_request'
+        run: python -m pip install "cpp-linter==${CPP_LINTER_VERSION}" "clang-format==${CLANG_TOOLS_VERSION}" "clang-tidy==${CLANG_TOOLS_VERSION}"


Match the pip-upgrade step used in test.yml so the linter job doesn't rely on the runner's bundled pip.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

+        run: |
+          cpp-linter \
+            --style=file \
+            --tidy-checks='' \
+            --version="${CLANG_TOOLS_VERSION%%.*}" \
+            --files-changed-only=true \
+            --lines-changed-only=true \
+            --thread-comments=true \
+            --ignore='build|cmake_modules|ci|src/iceberg/catalog/hive/gen-cpp' \
+            --database=build \
+            --verbosity=debug \
+            --extra-arg='-std=c++23' \
+            --extra-arg="-I${PWD}/src" \
+            --extra-arg="-I${PWD}/build/src" \
+            --extra-arg="-I${PWD}/build/_deps/sqlpp23-src/include" \
+            --extra-arg='-I/usr/include/postgresql' \
+            --extra-arg='-I/usr/include/mysql' \
+            --extra-arg='-fno-builtin-std-forward_like'


Decouple the two tools as CLANG_FORMAT_VERSION and CLANG_TIDY_VERSION so the format version can track .pre-commit-config.yaml (22.1.5) while clang-tidy uses 22.1.8, which is the nearest version published on PyPI. A comment marks the format version as the one to keep in sync.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.

+      # clang-format is kept in sync with .pre-commit-config.yaml; clang-tidy is
+      # pinned separately because that version is not published on PyPI.
+      CLANG_FORMAT_VERSION: "22.1.5"
+      CLANG_TIDY_VERSION: "22.1.8"


+        if: github.event_name == 'pull_request'
+        run: |
+          python -m pip install --upgrade pip
+          python -m pip install "cpp-linter==${CPP_LINTER_VERSION}" "clang-format==${CLANG_FORMAT_VERSION}" "clang-tidy==${CLANG_TIDY_VERSION}"


+        run: |
+          cpp-linter \
+            --style=file \
+            --tidy-checks='' \
+            --version="${CLANG_FORMAT_VERSION%%.*}" \
+            --files-changed-only=true \


+          python -m pip install --upgrade pip
+          python -m pip install "cpp-linter==${CPP_LINTER_VERSION}" "clang-format==${CLANG_FORMAT_VERSION}" "clang-tidy==${CLANG_TIDY_VERSION}"


wgtmac

Thinking more about this, I just realized that we do not need to duplicate the format check in cpp-linter. That should make our lives easier.

wgtmac · 2026-07-23T09:49:54Z

+        if: github.event_name == 'pull_request'
+        run: |
+          python -m pip install --upgrade pip
+          python -m pip install "cpp-linter==${CPP_LINTER_VERSION}" "clang-format==${CLANG_FORMAT_VERSION}" "clang-tidy==${CLANG_TIDY_VERSION}"


Do we need clang-format in this job at all? The pre-commit workflow already runs the pinned clang-format hook over all files on every PR. Keeping formatting there and making cpp-linter tidy-only would avoid duplicate checks and the version-sync problem. Could we drop CLANG_FORMAT_VERSION and the clang-format package here, and set --style to an empty string to disable format checks?

wgtmac · 2026-07-23T09:49:54Z

+          cpp-linter \
+            --style=file \
+            --tidy-checks='' \
+            --version="${CLANG_FORMAT_VERSION%%.*}" \


Even with a tidy-only setup, deriving --version=22 would still bypass the pip pin. cpp-linter looks for clang-tidy-22 first, and the probe run selected /usr/bin/clang-tidy-22 at 22.1.2 instead of the installed 22.1.8. Could we point this to ${pythonLocation}/bin, or omit --version, so the pip-installed binary is actually used?

Drop clang-format from the cpp-linter job (pre-commit already runs the pinned clang-format hook on every PR) and point --version at the setup-python bin dir so clang-tidy resolves to the pip-installed 22.1.8 instead of a system clang-tidy-22. Adds a temporary probe to exercise CI; will be dropped.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

.github/workflows/cpp-linter.yml:106

The workflow pins the top-level PyPI packages, but transitive dependencies can still drift over time, which may break CI reproducibility (and increases supply-chain surface). Consider installing from a locked requirements/constraints file (optionally with hashes) so the full dependency set is deterministic.

          python -m pip install --upgrade pip
          python -m pip install "cpp-linter==${CPP_LINTER_VERSION}" "clang-tidy==${CLANG_TIDY_VERSION}"

+// TEMP verification file — will be dropped before merge.
+// Confirms cpp-linter runs clang-tidy with the pip-installed binary
+// (via --version=${pythonLocation}/bin) after dropping clang-format.
+namespace iceberg {


Drop the probe added to exercise the tidy-only cpp-linter path in CI. CI confirmed clang-tidy resolves to the pip-installed 22.1.8 and reports the seeded violations.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

.github/workflows/cpp-linter.yml:101

Because this workflow’s pull_request.paths-ignore includes .github/**, a PR that only changes this workflow will not trigger the job. Now that the temporary probe file is gone, consider adding a temporary workflow_dispatch: trigger (or narrowing the ignore rules) so the final workflow-only change can be exercised in CI before merge.

      - uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6.3.0
        if: github.event_name == 'pull_request'
        with:
          python-version: '3.13'

+          cpp-linter \
+            --style='' \
+            --tidy-checks='' \


wgtmac · 2026-07-23T15:58:11Z

Nice work! Thanks @LuciferYang!

Copilot AI review requested due to automatic review settings July 22, 2026 06:33

LuciferYang mentioned this pull request Jul 22, 2026

Find alternative to cpp-linter/cpp-linter-action #336

Closed

Copilot started reviewing on behalf of LuciferYang July 22, 2026 06:34 View session

Copilot AI reviewed Jul 22, 2026

View reviewed changes

LuciferYang commented Jul 22, 2026

View reviewed changes

Comment thread src/iceberg/temp_linter_probe.cc Outdated

Copilot AI review requested due to automatic review settings July 22, 2026 06:51

LuciferYang force-pushed the replace-cpp-linter-action branch from 7889803 to 88d5abc Compare July 22, 2026 06:51

Copilot started reviewing on behalf of LuciferYang July 22, 2026 06:51 View session

Copilot AI reviewed Jul 22, 2026

View reviewed changes

ci: pin python-version and install via python -m pip

f8227d7

Address review feedback: pin setup-python to 3.13 so the runner's default Python cannot drift, and install with python -m pip so it targets the interpreter the action set up rather than a bare pip on PATH.

Copilot AI review requested due to automatic review settings July 22, 2026 06:58

Copilot started reviewing on behalf of LuciferYang July 22, 2026 06:59 View session

Copilot AI reviewed Jul 22, 2026

View reviewed changes

Comment thread .github/workflows/cpp-linter.yml

wgtmac approved these changes Jul 22, 2026

View reviewed changes

ci: parameterize cpp-linter and clang tool versions

6f02d43

Move the cpp-linter and clang tool versions into the job's env block so they live in one place, and derive the clang-tidy --version major from CLANG_TOOLS_VERSION instead of repeating it.

Copilot AI review requested due to automatic review settings July 23, 2026 05:00

Copilot started reviewing on behalf of LuciferYang July 23, 2026 05:00 View session

Copilot AI reviewed Jul 23, 2026

View reviewed changes

wgtmac reviewed Jul 23, 2026

View reviewed changes

Comment thread .github/workflows/cpp-linter.yml Outdated

Update .github/workflows/cpp-linter.yml

1989d0c

wgtmac reviewed Jul 23, 2026

View reviewed changes

Comment thread .github/workflows/cpp-linter.yml Outdated

Apply suggestion from @wgtmac

81c2d6f

Copilot AI review requested due to automatic review settings July 23, 2026 08:15

Copilot started reviewing on behalf of wgtmac July 23, 2026 08:15 View session

Copilot AI reviewed Jul 23, 2026

View reviewed changes

ci: upgrade pip before installing cpp-linter

3f52a62

Match the pip-upgrade step used in test.yml so the linter job doesn't rely on the runner's bundled pip.

Copilot AI review requested due to automatic review settings July 23, 2026 08:25

Copilot started reviewing on behalf of LuciferYang July 23, 2026 08:26 View session

Copilot AI reviewed Jul 23, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 23, 2026 08:32

Copilot started reviewing on behalf of wgtmac July 23, 2026 08:34 View session

Copilot AI reviewed Jul 23, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 23, 2026 09:02

Copilot AI reviewed Jul 23, 2026

View reviewed changes

Copilot started reviewing on behalf of LuciferYang July 23, 2026 09:23 View session

wgtmac reviewed Jul 23, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 23, 2026 10:53

Copilot started reviewing on behalf of LuciferYang July 23, 2026 10:57 View session

Copilot AI reviewed Jul 23, 2026

View reviewed changes

test: remove temporary linter probe

d679da7

Drop the probe added to exercise the tidy-only cpp-linter path in CI. CI confirmed clang-tidy resolves to the pip-installed 22.1.8 and reports the seeded violations.

Copilot AI review requested due to automatic review settings July 23, 2026 11:20

Copilot started reviewing on behalf of LuciferYang July 23, 2026 11:21 View session

Copilot AI reviewed Jul 23, 2026

View reviewed changes

Comment thread .github/workflows/cpp-linter.yml

Comment on lines +115 to +117

cpp-linter \

--style='' \

--tidy-checks='' \

wgtmac merged commit f9e83ff into apache:main Jul 23, 2026
20 checks passed

wgtmac mentioned this pull request Jul 23, 2026

ci: share sccache setup across workflows #811

Open

		// TEMP verification file — will be dropped before merge.
		// Intentionally violates .clang-tidy to confirm the new cpp-linter CLI runs.

		python -m pip install --upgrade pip
		python -m pip install "cpp-linter==${CPP_LINTER_VERSION}" "clang-format==${CLANG_FORMAT_VERSION}" "clang-tidy==${CLANG_TIDY_VERSION}"

Uh oh!

Conversation

LuciferYang commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

How

Verified

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LuciferYang Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

LuciferYang commented Jul 22, 2026

Uh oh!

wgtmac left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

wgtmac left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

LuciferYang commented Jul 22, 2026 •

edited

Loading

LuciferYang Jul 22, 2026 •

edited

Loading